Avoid redundant broadcast of local commitment transaction #1922
Conversation
This change follows the rationale of commit 62236c7 and addresses the last remaining redundant local commitment broadcast. There's no need to broadcast our local commitment transaction if we've already seen a confirmed one as it'll be immediately rejected as a duplicate/conflict. This will also help prevent dispatching spurious events for bumping commitment and HTLC transactions through anchor outputs since the dispatch for said events follows the same flow as our usual commitment broadcast.
Codecov ReportBase: 90.73% // Head: 90.73% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1922 +/- ##
==========================================
- Coverage 90.73% 90.73% -0.01%
==========================================
Files 94 94
Lines 49603 49538 -65
Branches 49603 49538 -65
==========================================
- Hits 45006 44947 -59
+ Misses 4597 4591 -6
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
Only mostly related to this PR, but should we not also have a check like the one added here in |
There was a problem hiding this comment.
LGTM.
Nice! I just recently ran into the issue of duplicate commitment broadcasts when testing force closes in LDK Lite (see lightningdevkit/ldk-node#33 (comment)). Of course, this won't fix the issue there, as we still would try to broadcast when we don't see the commitment on chain, but still nice to have!
This change follows the rationale of commit 62236c7 and addresses the last remaining redundant local commitment broadcast.
There's no need to broadcast our local commitment transaction if we've already seen a confirmed one as it'll be immediately rejected as a duplicate/conflict.
This will also help prevent dispatching spurious events for bumping commitment and HTLC transactions through anchor outputs since the dispatch for said events follows the same flow as our usual commitment broadcast.
The bulk of this patch is addressing a series of test failures resulting from the broadcast logic change.